-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
功能: Table 支持 Cell 合并 #31
Conversation
WalkthroughThe recent changes to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TableEditor
participant DeleteCol
participant InsertCol
participant MergeCell
participant SplitCell
participant RenderTable
participant Plugin
User->>TableEditor: Delete Column
TableEditor->>DeleteCol: Execute delete column logic
DeleteCol-->>TableEditor: Updated table structure
User->>TableEditor: Insert Column
TableEditor->>InsertCol: Execute insert column logic
InsertCol-->>TableEditor: Updated table structure
User->>TableEditor: Merge Cells
TableEditor->>MergeCell: Execute merge cell logic
MergeCell-->>TableEditor: Merged cells
User->>TableEditor: Split Cells
TableEditor->>SplitCell: Execute split cell logic
SplitCell-->>TableEditor: Split cells
TableEditor->>RenderTable: Render updated table
RenderTable-->>User: Display updated table
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (34)
- packages/basic-modules/src/modules/image/render-elem.tsx (8 hunks)
- packages/core/src/editor/dom-editor.ts (1 hunks)
- packages/core/src/text-area/syncSelection.ts (1 hunks)
- packages/editor/src/init-default-config/config/hoverbar.ts (1 hunks)
- packages/table-module/package.json (1 hunks)
- packages/table-module/src/assets/index.less (3 hunks)
- packages/table-module/src/constants/svg.ts (1 hunks)
- packages/table-module/src/locale/en.ts (1 hunks)
- packages/table-module/src/locale/zh-CN.ts (1 hunks)
- packages/table-module/src/module/column-resize.ts (1 hunks)
- packages/table-module/src/module/custom-types.ts (2 hunks)
- packages/table-module/src/module/elem-to-html.ts (2 hunks)
- packages/table-module/src/module/index.ts (2 hunks)
- packages/table-module/src/module/menu/DeleteCol.ts (2 hunks)
- packages/table-module/src/module/menu/DeleteRow.ts (2 hunks)
- packages/table-module/src/module/menu/InsertCol.ts (2 hunks)
- packages/table-module/src/module/menu/InsertRow.ts (2 hunks)
- packages/table-module/src/module/menu/InsertTable.ts (2 hunks)
- packages/table-module/src/module/menu/MergeCell.ts (1 hunks)
- packages/table-module/src/module/menu/SplitCell.ts (1 hunks)
- packages/table-module/src/module/menu/index.ts (2 hunks)
- packages/table-module/src/module/plugin.ts (2 hunks)
- packages/table-module/src/module/render-elem/render-cell.tsx (2 hunks)
- packages/table-module/src/module/render-elem/render-table.tsx (4 hunks)
- packages/table-module/src/module/table-cursor.ts (1 hunks)
- packages/table-module/src/module/weak-maps.ts (1 hunks)
- packages/table-module/src/module/with-selection.ts (1 hunks)
- packages/table-module/src/utils/has-common.ts (1 hunks)
- packages/table-module/src/utils/index.ts (1 hunks)
- packages/table-module/src/utils/is-of-type.ts (1 hunks)
- packages/table-module/src/utils/matrices.ts (1 hunks)
- packages/table-module/src/utils/options.ts (1 hunks)
- packages/table-module/src/utils/point.ts (1 hunks)
- packages/table-module/src/utils/types.ts (1 hunks)
Files skipped from review due to trivial changes (3)
- packages/core/src/text-area/syncSelection.ts
- packages/table-module/src/module/weak-maps.ts
- packages/table-module/src/utils/index.ts
Additional context used
Biome
packages/table-module/src/utils/point.ts
[error] 11-11: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
packages/table-module/src/module/menu/InsertCol.ts
[error] 68-68: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 57-57: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 68-68: This code is unreachable
... because this statement will break the flow of the code beforehand
(lint/correctness/noUnreachable)
packages/table-module/src/module/with-selection.ts
[error] 16-17: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 26-26: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 41-41: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 49-49: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
packages/table-module/src/module/menu/MergeCell.ts
[error] 121-121: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
packages/table-module/src/module/menu/DeleteCol.ts
[error] 76-76: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 65-65: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 76-76: This code is unreachable
... because this statement will break the flow of the code beforehand
(lint/correctness/noUnreachable)
packages/table-module/src/module/render-elem/render-table.tsx
[error] 142-143: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
[error] 155-156: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
Additional comments not posted (79)
packages/table-module/src/locale/zh-CN.ts (1)
16-17
: Localization strings added for cell merging and splitting.The added localization strings for 'merge cell' and 'split cell' are appropriate and consistent with the existing entries.
packages/table-module/src/utils/point.ts (1)
1-17
: Class definition and methods look good.The
Point
class is well-defined and the methods are correct.Tools
Biome
[error] 11-11: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
packages/table-module/src/utils/types.ts (1)
1-22
: Type definitions look good.The type definitions for
CellElement
,WithType
,NodeEntryWithContext
,SelectionMode
, andEdge
are correct and clear.packages/table-module/src/utils/options.ts (3)
1-3
: Import statements look good.The import statements correctly bring in necessary types from 'slate'.
5-16
: InterfaceWithTableOptions
is well-defined.The interface
WithTableOptions
defines the structure for table-related block elements, which is essential for table functionalities.
18-29
: Default table options are correctly set.The
DEFAULT_WITH_TABLE_OPTIONS
object provides default values for table-related block elements, ensuring consistency across the application.packages/table-module/src/utils/is-of-type.ts (3)
1-4
: Import statements look good.The import statements correctly bring in necessary modules and types.
5-7
: FunctionisElement
is well-implemented.The function
isElement
correctly checks if a node is an element and includes a 'type' property.
9-17
: FunctionisOfType
is well-implemented.The function
isOfType
generates aNodeMatch
function to match elements of specific types, which is essential for table functionalities.packages/table-module/src/utils/has-common.ts (2)
1-4
: Import statements look good.The import statements correctly bring in necessary modules and types.
5-26
: FunctionhasCommon
is well-implemented.The function
hasCommon
correctly determines if two paths share a common ancestor node of a specific type, which is essential for table functionalities.packages/table-module/src/module/custom-types.ts (2)
17-19
: Addition ofhidden
property inTableCellElement
is appropriate.The
hidden
property is added toTableCellElement
for setting the display attribute, which is essential for managing cell visibility.
31-38
: Addition of new properties inTableElement
is appropriate.The new properties (
scrollWidth
,height
,resizingIndex
,isResizing
,isHoverCellBorder
,columnWidths
) are added toTableElement
for managing table layout and resizing functionalities.packages/table-module/src/module/elem-to-html.ts (2)
12-12
: Approve the addition oftable-layout: fixed
intableToHtml
.The addition of
table-layout: fixed
ensures consistent layout for tables with merged cells.
25-29
: Approve the handling of hidden cells intableCellToHtml
.The changes correctly handle hidden cells by adding a
display:none
style when thehidden
property is true.packages/editor/src/init-default-config/config/hoverbar.ts (2)
36-36
: Approve the addition ofmergeTableCell
menu key.The addition of
mergeTableCell
menu key is necessary for the merge cell functionality in the hoverbar menu.
37-37
: Approve the addition ofsplitTableCell
menu key.The addition of
splitTableCell
menu key is necessary for the split cell functionality in the hoverbar menu.packages/table-module/src/module/index.ts (2)
21-21
: Approve the addition ofmergeTableCellConf
.The addition of
mergeTableCellConf
is necessary for the merge cell functionality in the table module.
22-22
: Approve the addition ofsplitTableCellConf
.The addition of
splitTableCellConf
is necessary for the split cell functionality in the table module.packages/table-module/package.json (1)
45-45
: Approve the addition oflodash.debounce
as a peer dependency.The addition of
lodash.debounce
as a peer dependency is necessary for handling debounce functionality in the table module.packages/table-module/src/module/menu/index.ts (3)
14-15
: Imports look good.The new imports for
MergeCell
andSplitCell
are necessary for the new menu configurations.
74-79
: New configuration looks good.The
mergeTableCellConf
configuration follows the pattern of other menu configurations.
81-86
: New configuration looks good.The
splitTableCellConf
configuration follows the pattern of other menu configurations.packages/table-module/src/module/render-elem/render-cell.tsx (3)
12-12
: Import looks good.The new import for
TableCursor
is necessary for the new logic handling cell selection.
27-36
: Logic for handling hidden cells looks good.The logic correctly handles hidden cells and selection in non-first row cells.
50-57
: Logic for handling hidden cells looks good.The logic correctly handles hidden cells and selection in first row cells.
packages/table-module/src/assets/index.less (2)
16-16
: Style change looks good.The style sets the table layout to fixed, which is necessary for handling column resizing and hidden cells.
50-88
: Styles for column resizing and selection look good.The styles correctly handle column resizing and selection highlighting.
packages/table-module/src/utils/matrices.ts (2)
1-42
: Function looks good.The
matrices
function correctly generates matrices for table sections.
44-88
: Function looks good.The
filledMatrix
function correctly generates filled matrices for table sections.packages/table-module/src/module/table-cursor.ts (4)
18-24
: LGTM!The function
isInTable
correctly checks if the selection is inside a table.
30-43
: LGTM!The function
selection
correctly retrieves a matrix representing the selected cells within a table.
46-75
: LGTM!The function
unselect
correctly clears the selection from the table and the browser selection.
81-88
: LGTM!The function
isSelected
correctly checks if a given cell is part of the current table selection.packages/table-module/src/module/menu/InsertRow.ts (4)
Line range hint
15-17
:
LGTM!The method
getValue
correctly returns an empty string as no value is needed.
Line range hint
19-21
:
LGTM!The method
isActive
correctly returnsfalse
as no active state is needed.
Line range hint
23-33
:
LGTM!The method
isDisabled
correctly checks if the insert row operation should be disabled based on the current selection.
Line range hint
35-112
:
LGTM!The method
exec
correctly performs the insert row operation, handling cell merging and inserting a new row at the correct position.packages/table-module/src/module/menu/DeleteRow.ts (4)
Line range hint
13-15
:
LGTM!The method
getValue
correctly returns an empty string as no value is needed.
Line range hint
17-19
:
LGTM!The method
isActive
correctly returnsfalse
as no active state is needed.
Line range hint
21-31
:
LGTM!The method
isDisabled
correctly checks if the delete row operation should be disabled based on the current selection.
Line range hint
33-118
:
LGTM!The method
exec
correctly performs the delete row operation, handling cell merging and deleting the selected row at the correct position.packages/table-module/src/module/menu/InsertCol.ts (4)
Line range hint
15-17
:
LGTM!The method
getValue
correctly returns an empty string as no value is needed.
Line range hint
19-21
:
LGTM!The method
isActive
correctly returnsfalse
as no active state is needed.
Line range hint
23-33
:
LGTM!The method
isDisabled
correctly checks if the insert column operation should be disabled based on the current selection.
Line range hint
35-123
:
LGTM!The method
exec
correctly performs the insert column operation, handling cell merging and inserting a new column at the correct position.Tools
Biome
[error] 68-68: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 57-57: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 68-68: This code is unreachable
... because this statement will break the flow of the code beforehand
(lint/correctness/noUnreachable)
packages/table-module/src/module/with-selection.ts (3)
9-13
: Ensure the selection logic is correct and efficient.The logic for handling selection operations is well-implemented, ensuring that the selection remains active when dragging cell widths. However, verify that this does not introduce any performance issues or unexpected behaviors.
52-98
: Ensure the bounds calculation is accurate and efficient.The nested loops for calculating the initial bounds and expanding the selection based on rowspan and colspan are correct. However, ensure that this logic is efficient and does not cause performance degradation for large tables.
100-117
: Verify the correctness of the selection state updates.The logic for updating the editor's selection state using
EDITOR_TO_SELECTION
andEDITOR_TO_SELECTION_SET
appears correct. Ensure that these updates accurately reflect the user's selection and do not introduce any inconsistencies.packages/table-module/src/module/menu/MergeCell.ts (4)
14-17
: LGTM!The
getValue
method correctly returns an empty string as no value needs to be retrieved.
19-22
: LGTM!The
isActive
method correctly returns false as there is no need to check for active state.
24-26
: Ensure thecanMerge
method is accurate.The
isDisabled
method relies on thecanMerge
method to determine if merging is possible. Verify the correctness of thecanMerge
method to ensure this logic is accurate.
121-121
: Remove the unnecessarycontinue
statement.The
continue
statement is not required here and can be removed to simplify the code.- continue
Likely invalid or redundant comment.
Tools
Biome
[error] 121-121: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
packages/table-module/src/module/menu/DeleteCol.ts (4)
Line range hint
14-17
:
LGTM!The
getValue
method correctly returns an empty string as no value needs to be retrieved.
Line range hint
19-22
:
LGTM!The
isActive
method correctly returns false as there is no need to check for active state.
Line range hint
24-36
:
LGTM!The
isDisabled
method correctly determines if the current selection is valid for deleting a column.
76-76
: Add a semicolon after the statement.An explicit or implicit semicolon is expected here to end the statement.
- for (let x = 0; x < matrix.length; x++) { + for (let x = 0; x < matrix.length; x++); {Likely invalid or redundant comment.
Tools
Biome
[error] 76-76: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 76-76: This code is unreachable
... because this statement will break the flow of the code beforehand
(lint/correctness/noUnreachable)
packages/table-module/src/module/menu/SplitCell.ts (4)
13-16
: LGTM!The
getValue
method correctly returns an empty string as no value needs to be retrieved.
18-21
: LGTM!The
isActive
method correctly returns false as there is no need to check for active state.
23-35
: LGTM!The
isDisabled
method correctly determines if the current selection can be split based on rowspan and colspan.
51-145
: LGTM!The
split
method correctly handles the cell splitting operation, updating the editor's state and managing various edge cases.packages/table-module/src/module/menu/InsertTable.ts (2)
16-16
: LGTM!The addition of the
columnWidths
array with a default width of 60 for each column is correct and consistent with the function's logic.
42-42
: LGTM!The addition of the
columnWidths
array to the returned table node is correct and consistent with the function's logic.packages/table-module/src/module/plugin.ts (1)
226-230
: LGTM!The addition of the
withSelection
function to handle selection behaviors is correct and consistent with the rest of the function.packages/table-module/src/module/render-elem/render-table.tsx (1)
58-66
: LGTM!The addition of the new properties to the table element is correct and consistent with the rest of the function's logic.
packages/basic-modules/src/modules/image/render-elem.tsx (5)
36-37
: LGTM!The change to comment out the CSS property for height is correct and consistent with the function's logic.
64-64
: LGTM!The addition of the
maxWidth
variable to handle image width adjustments is correct and consistent with the function's logic.
120-120
: LGTM!The change to comment out the CSS property for height is correct and consistent with the function's logic.
129-129
: LGTM!The change to comment out the CSS property for height is correct and consistent with the function's logic.
147-147
: LGTM!The change to comment out the CSS property for height is correct and consistent with the function's logic.
packages/table-module/src/constants/svg.ts (2)
45-46
: LGTM! Verify SVG string length.The
MERGE_CELL_SVG
constant addition looks good. Ensure that the SVG string length is optimized to avoid increasing the code size unnecessarily.
49-50
: LGTM! Verify SVG string length.The
SPLIT_CELL_SVG
constant addition looks good. Ensure that the SVG string length is optimized to avoid increasing the code size unnecessarily.packages/table-module/src/module/column-resize.ts (6)
11-21
: LGTM!The
getCumulativeWidths
function correctly calculates the cumulative widths of columns.
26-35
: LGTM!The
getColumnWidthRatios
function correctly calculates the width ratios of columns.
42-60
: LGTM! Verify memory management.The
observerTableResize
function correctly sets up aResizeObserver
to observe table size changes. Ensure that the observer is properly disconnected to avoid memory leaks.
62-67
: LGTM!The
unObserveTableResize
function correctly disconnects theResizeObserver
to prevent memory leaks.
79-113
: LGTM! Verify edge cases.The
onMouseDown
function correctly handles the mousedown event for column resizing. Ensure that all edge cases are properly handled.
115-149
: LGTM!The
onMouseMove
function correctly handles the mousemove event for column resizing.packages/core/src/editor/dom-editor.ts (1)
219-221
: LGTM! Verify necessity of new condition.The modification to the
hasDOMNode
function to include an additional condition checking for the attributedata-slate-string
looks good. Ensure that this new condition is necessary and properly integrated.
收到,今晚 review。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/table-module/src/locale/en.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/table-module/src/locale/en.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
代码看了一遍,涉及table的改动挺大,只能是粗略的过了一遍,关于 render-elem 和
syncSelection 的改动应该没有影响,table 也测过了没有问题。
现在比较关键的是单测需要做适配更改。这个你有空看一下吗,其它模块的单测都是toBe的字符串匹配问题,table模块单测有两个空值引用问题。
需要的话 ,toBe 比较我可以提 pr 做适配,你可以看一下 修复一下 两个空值的问题。
需要跑一下 yarn lint --fix |
我这边check到你的分支改了部分 test 文件,因为 lint 校验没过,提不上去,没法合到你的分支里,这太扯了😂 |
确实没有 lint; table 单测完看看。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (13)
- packages/basic-modules/tests/image/render-elem.test.ts (2 hunks)
- packages/core/src/editor/dom-editor.ts (1 hunks)
- packages/table-module/tests/elem-to-html.test.ts (3 hunks)
- packages/table-module/tests/render-elem.test.ts (1 hunks)
- packages/table-module/src/constants/svg.ts (1 hunks)
- packages/table-module/src/module/elem-to-html.ts (2 hunks)
- packages/table-module/src/module/menu/DeleteCol.ts (2 hunks)
- packages/table-module/src/module/menu/InsertCol.ts (2 hunks)
- packages/table-module/src/module/menu/InsertTable.ts (2 hunks)
- packages/table-module/src/module/menu/SplitCell.ts (1 hunks)
- packages/table-module/src/module/plugin.ts (2 hunks)
- packages/table-module/src/module/render-elem/render-cell.tsx (2 hunks)
- packages/table-module/src/module/render-elem/render-table.tsx (4 hunks)
Files skipped from review due to trivial changes (1)
- packages/table-module/src/module/plugin.ts
Files skipped from review as they are similar to previous changes (5)
- packages/core/src/editor/dom-editor.ts
- packages/table-module/src/module/elem-to-html.ts
- packages/table-module/src/module/menu/InsertTable.ts
- packages/table-module/src/module/menu/SplitCell.ts
- packages/table-module/src/module/render-elem/render-cell.tsx
Additional context used
Biome
packages/table-module/src/module/menu/InsertCol.ts
[error] 68-68: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 57-57: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 68-68: This code is unreachable
... because this statement will break the flow of the code beforehand
(lint/correctness/noUnreachable)
packages/table-module/src/module/menu/DeleteCol.ts
[error] 76-76: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 65-65: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 76-76: This code is unreachable
... because this statement will break the flow of the code beforehand
(lint/correctness/noUnreachable)
packages/table-module/src/module/render-elem/render-table.tsx
[error] 146-146: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
[error] 159-160: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
Additional comments not posted (15)
packages/table-module/__tests__/render-elem.test.ts (2)
37-43
: Accurate and well-documented structural change.The test case accurately checks the rendering of the table element with the new outer div structure. The added comment provides clear context for this change.
51-53
: Correct test case for full-width table rendering.The test case correctly verifies that the table element is rendered with a width of 100% when specified.
packages/basic-modules/__tests__/image/render-elem.test.ts (2)
36-36
: Correct addition of style property to image element.The style property specifies the width of the image. The test case correctly verifies the rendering of the image element with this style.
44-44
: Temporary disabling of height style property check.The commented-out line indicates that the test case for the height style property is temporarily disabled due to an issue. This is acceptable if the issue is tracked and resolved later.
packages/table-module/__tests__/elem-to-html.test.ts (2)
Line range hint
62-92
: Correct test case for converting table cell to HTML.The test case correctly verifies the conversion of a table cell element to an HTML string with the correct attributes and content.
102-104
: Correct test case for converting full-width table to HTML.The test case correctly verifies the conversion of a full-width table element to an HTML string with the correct style attribute.
packages/table-module/src/module/menu/InsertCol.ts (4)
6-11
: Necessary imports for enhanced logic.The imports for
Path
fromslate
andfilledMatrix
from../../utils
are necessary for the enhanced logic for handling cell insertion and merging.
55-67
: Correct logic for determining the selected cell index.The logic correctly determines the index of the selected cell in the matrix for further operations.
Tools
Biome
[error] 57-57: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
68-89
: Insert new cells and handle merged cells correctly.The logic ensures that new cells are inserted correctly and merged cells are handled appropriately.
Tools
Biome
[error] 68-68: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 68-68: This code is unreachable
... because this statement will break the flow of the code beforehand
(lint/correctness/noUnreachable)
108-119
: Adjust column widths correctly.The logic ensures that column widths are adjusted appropriately after inserting new cells.
packages/table-module/src/module/menu/DeleteCol.ts (2)
9-10
: New imports are appropriate.The new imports
filledMatrix
from../../utils
andTableCellElement
,TableElement
from../custom-types
are necessary for the enhanced logic of deleting columns in tables.
75-76
: Add a missing semicolon.An explicit or implicit semicolon is expected here to end the statement.
- Editor.withoutNormalizing(editor, () => { + Editor.withoutNormalizing(editor, () => { + ;Likely invalid or redundant comment.
Tools
Biome
[error] 76-76: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 76-76: This code is unreachable
... because this statement will break the flow of the code beforehand
(lint/correctness/noUnreachable)
packages/table-module/src/module/render-elem/render-table.tsx (2)
6-19
: New imports are appropriate.The new imports
debounce
fromlodash.debounce
,TableCursor
from../table-cursor
, and several functions from../column-resize
are necessary for the enhanced logic of rendering table cells, handling cell borders, and column resizing.
68-73
: Ensure the correct handling of table selection.The variable
selected
is assigned usingDomEditor.isNodeSelected
, and the variableisSelecting
is assigned usingTableCursor.selection
. Ensure that the logic correctly handles table selection and cursor behavior.packages/table-module/src/constants/svg.ts (1)
45-50
: New SVG icons are appropriate.The new SVG icons
MERGE_CELL_SVG
andSPLIT_CELL_SVG
are necessary for the new functionality of merging and splitting table cells.
哈哈哈,我就是这个有点困惑才让你看下的,我今天有事应该看不了这个了,明天看看能不能解决,不能解决的话就小步快跑了。说不定看半天是jest的毛病呢,我有打算换 vtest 的,现在单测有点多,vtest 似乎更快一些。 |
我在给 jest打断点,找到问题了,我改改 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Outside diff range, codebase verification and nitpick comments (1)
tests/setup/index.ts (1)
26-40
: Ensure complete implementation of ResizeObserver mock.The
ResizeObserver
mock methods are currently placeholders. If specific behavior is required for tests, consider implementing the methods or adding comments to clarify their purpose.observe() { // 可以根据需要添加具体实现 } unobserve() { // 可以根据需要添加具体实现 } disconnect() { // 可以根据需要添加具体实现 }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- packages/table-module/tests/menu/delete-col.test.ts (2 hunks)
- packages/table-module/tests/menu/delete-row.test.ts (3 hunks)
- packages/table-module/tests/menu/insert-col.test.ts (2 hunks)
- packages/table-module/src/module/menu/DeleteCol.ts (2 hunks)
- packages/table-module/src/module/menu/InsertCol.ts (2 hunks)
- tests/setup/index.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- packages/table-module/tests/menu/insert-col.test.ts
Additional context used
Biome
packages/table-module/src/module/menu/InsertCol.ts
[error] 68-68: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 57-57: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 68-68: This code is unreachable
... because this statement will break the flow of the code beforehand
(lint/correctness/noUnreachable)
packages/table-module/src/module/menu/DeleteCol.ts
[error] 76-76: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 65-65: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 76-76: This code is unreachable
... because this statement will break the flow of the code beforehand
(lint/correctness/noUnreachable)
Additional comments not posted (8)
packages/table-module/src/module/menu/InsertCol.ts (1)
11-11
: ImportfilledMatrix
.The import statement for
filledMatrix
has been added, which is used in theexec
method.packages/table-module/src/module/menu/DeleteCol.ts (1)
9-10
: ImportfilledMatrix
.The import statement for
filledMatrix
has been added, which is used in theexec
method.packages/table-module/__tests__/menu/delete-row.test.ts (2)
9-11
: MockfilledMatrix
.The
filledMatrix
function is mocked for testing purposes.
Line range hint
143-156
:
Ensure test coverage for different scenarios.The test case for
exec
method includes scenarios for removing rows. Ensure that all edge cases are covered, such as tables with merged cells or different structures.packages/table-module/__tests__/menu/delete-col.test.ts (4)
4-4
: Import statement approved.The import of
utils
is correctly added.
9-11
: MockingfilledMatrix
function is correct.The
filledMatrix
function is mocked correctly usingjest.fn()
.
12-13
: Mocked utils assignment approved.The assignment of
mockedUtils
is correct.
148-151
: Verify the correctness of the mockedfilledMatrix
implementation.The
filledMatrix
mock implementation returns a nested array representing the table structure. Verify the correctness of this structure to ensure it matches the expected format.Verification successful
Verify the correctness of the mocked
filledMatrix
implementation.The mocked
filledMatrix
implementation in the test file accurately reflects the structure produced by the actualfilledMatrix
function. The nested arrays, node entries, and context information (rtl, ltr, ttb, btt) are consistent with the expected format.
- The mocked implementation returns a nested array structure with node entries and context information that matches the real implementation of
filledMatrix
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of the mocked filledMatrix implementation. # Test: Ensure the returned structure matches the expected table format. rg --type ts 'function filledMatrix' -A 10Length of output: 934
Script:
#!/bin/bash # Description: Verify the correctness of the mocked filledMatrix implementation. # Retrieve the complete implementation of the filledMatrix function. rg --type ts 'function filledMatrix' -A 50Length of output: 3273
已经发布了新版本,Cypress 的 action checkout,出问题了,我晚上回去看一下。 |
嗯嗯,什么问题 |
看了一下,似乎是 e2e 测试的 docker 容器版本过时了,换了最新版和node版本,现在没问题了。 |
@jschyz 方便加个微信不?ylf2991205548 |
实现参考
https://github.com/nlulic/slate-table
飞书表格
WangEditor 支持 Table Cell 合并 [预览]
基础能力
列拖拽
行列新增 / 删除
宽度自适应
图片自适应
Summary by CodeRabbit
New Features
Bug Fixes
Refactor